fiftyone/server/lightning.py (1)

331-333: Improve error handling for distinct query failures

The current try-except block catches all exceptions without specific handling or logging.

    try:
        result = await collection.distinct(query.path, filter)
-   except:
+   except Exception as e:
+       logger.warning(f"Distinct query failed for path {query.path}: {str(e)}")
        # too many results
        return None

Also applies to: 340-340

app/schema.graphql (1)

447-447: Consider adding schema documentation

While the implementation is solid, consider adding GraphQL descriptions to document the purpose and usage of the new slice field. This would improve schema discoverability and maintainability.

Example addition:

input LightningInput {
  dataset: String!
  paths: [LightningPathInput!]!
+ """
+ Filter results to a specific slice of a group dataset.
+ When null, no slice filtering is applied.
+ """
  slice: String = null
}

tests/unittests/lightning_tests.py (3)

1056-1139: Consider adding edge cases to the test suite.

The test implementation looks good but could be more comprehensive. Consider adding test cases for:

• Empty slice names
• Non-existent slice names
• Multiple samples in the same slice

---

Line range hint 1155-1170: Add type hints and docstring for the new parameters.

The function implementation looks good, but could benefit from better documentation.

Consider adding a docstring and improving type hints:

 async def _execute(
     query: str,
     dataset: fo.Dataset,
     field: fo.Field,
     keys: t.Set[str],
-    frames=True,
-    slice: t.Optional[str] = None,
+    frames: bool = True,
+    slice: t.Optional[str] = None,
 ):
+    """Execute a GraphQL query on the dataset.
+    
+    Args:
+        query: The GraphQL query string
+        dataset: The FiftyOne dataset
+        field: The field type to query
+        keys: Set of field keys to include
+        frames: Whether to include frame fields
+        slice: Optional slice name to filter the dataset
+    
+    Returns:
+        The query execution result
+    """

---

1178-1194: Add docstring for the _get_paths function.

The function implementation is correct, but would benefit from documentation.

Consider adding a docstring:

 def _get_paths(
     dataset: fo.Dataset,
     field_type: t.Type[fo.Field],
     keys: t.Set[str],
     frames: bool = True,
 ):
+    """Get the field paths for the dataset.
+    
+    Args:
+        dataset: The FiftyOne dataset
+        field_type: The type of field to include
+        keys: Set of field keys to include
+        frames: Whether to include frame fields
+    
+    Returns:
+        List of LightningPathInput objects
+    """

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Description: Verify that the GraphQL schema includes the slice field in LightningInput type

# Check if the schema file exists and contains the required field
rg -U "type LightningInput \{(\n.*?)*slice:" app/schema.graphql

#!/bin/bash
# Let's first find the GraphQL schema files in the repository
fd "schema.*graphql"

# Then let's search for LightningInput type definition in any GraphQL files
rg -U "type\s+LightningInput\s*\{[^}]*\}" --type graphql

# Also search for any input type that has a slice field
rg -U "input\s+\w+\s*\{[^}]*slice:[^}]*\}" --type graphql

# Let's also check the TypeScript interface/type definitions related to Lightning
rg -U "interface\s+Lightning|type\s+Lightning" --type typescript

#!/bin/bash
# Let's check the content of the main schema file
cat app/schema.graphql

# Also search for any input types with slice field without specifying file type
rg -U "input\s+\w+\s*\{[^}]*slice:[^}]*\}"

# Search for Lightning related types without specifying file type
rg -U "type\s+Lightning|input\s+Lightning"

    filter = (
+       {f"{dataset.group_field}.name": input.slice}
+       if dataset.group_field and input.slice and input.slice in dataset.distinct(f"{dataset.group_field}.name")
+       else None
-       {f"{dataset.group_field}.name": input.slice}
-       if dataset.group_field and input.slice
-       else None
    )

#!/bin/bash
# Description: Search for group validation logic in the codebase
# Look for references to group validation or group name retrieval
rg -g '!*.pyc' -g '!*test*' "group.*names|available.*groups" 

#!/bin/bash
# Search for distinct method implementation and its usage with group fields
ast-grep --pattern 'def distinct'

# Also check for any existing validation of group fields
rg -g '!*.pyc' "group_field.*valid|validate.*group" -A 3

#!/bin/bash
# Search for distinct method implementation in dataset or collection classes
rg -g '!*.pyc' "def distinct.*group" -A 5

# Also check for any existing usage of distinct with group fields
rg -g '!*.pyc' "distinct.*group.*name" -A 2

#!/bin/bash
# Search for distinct method implementation in dataset or collection classes
rg -g '!*.pyc' "def distinct" -A 5

# Also check for any MongoDB-related distinct operations
rg -g '!*.pyc' "mongo.*distinct" -A 2

#!/bin/bash
# Description: Check for consistent usage of the new slice parameter across the codebase

# Search for LightningInput usage to ensure proper handling of the new slice parameter
ast-grep --pattern 'new LightningInput($$$)' 

# Search for references to group dataset filtering with slice
rg -l 'group.*dataset.*slice|slice.*group.*dataset'

#!/bin/bash
# Let's check the implementation details around the slice parameter usage

# Check mutation handling of slice parameter
rg -A 5 "slice.*LightningInput" app/

# Look for GraphQL query definitions using slice
rg -A 5 "slice.*group.*dataset" app/

# Check server-side handling in mutation.py
cat fiftyone/server/mutation.py